ChatTrigger: Completely rework the criteria and parameter systems #122
Conversation
This removes the `Parameter` class and replaces it with 3 methods: `startsWith`, `contains`, and `endsWith`. These methods all take in criterias to make the API more understandable to users (in the past, I've noticed a lot of people attempting to do `setContains(msg)` instead of `setCriteria(msg).setContains()`).
setCriteria will work as normal, with a few slight tweaks.
- Now global flags will work if the criteria is passed as a Regex
- `.` or criteria variables will now detect `\n`.
- String criteria variables will now not be greedy. e.g. `setContains("<${a}>")` with a string of "<A> B>" will capture "A" in this string, instead of "A> B".
|
What are the breaking changes here? I like the idea, but this is an extremely hot area of code, and can cause a massive amount of breakage which I'd like to avoid if possible. For example, I think the criteria greediness should be opt-in, e.g., something like |
Breaking changes:
The greediness opt-in sounds good to me, but what kind of symbol should we use to denote capturing newlines? Or would we have a boolean second param or something. |
This removes the
Parameterclass and replaces it with 3 methods:startsWith,contains, andendsWith. These methods all take in criterias to make the API more understandable to users (in the past, I've noticed a lot of people attempting to dosetContains(msg)instead ofsetCriteria(msg).setContains()).setCriteria will work as normal, with a few slight tweaks.
.or criteria variables will now detect\n.setContains("<${a}>")with a string of "<A> B>" will capture "A" in this string, instead of "A> B".A few thoughts:
I'm thinking that the
containscriteria maybe should be global by default when using string criteriaThere should be some way to do a "contains or" instead of having every contains match. Right now
contains()has to have all of the containing criterias matchcontainsAll(),containsAny(),containsNone()startsWithout,endsWithout?